Skip to content

Conversation

M-i-k-e-l
Copy link
Collaborator

Description

Incubator.Gradient - new component
Ideally this would be using something other than react-native-linear-gradient, however I did not manage to get all the features to work consistently.

Changelog

Incubator.Gradient - new component

Additional info

None

centerV?: boolean;
};

// type GradientType = 'rectangle' | 'circle' | 'border';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be removed

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hoped to fix the expect-error in the screen, kinda still want to keep this

@@ -0,0 +1,35 @@
import type {LinearGradientProps} from 'react-native-linear-gradient';

type CommonGradientProps = Pick<LinearGradientProps, 'colors' | 'children'> & {
Copy link
Contributor

@adids1221 adids1221 Aug 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LinearGradientProps has start & end props (link), I think the user should be able to control them.
Nice work with the angle prop but playing with start & end can give you more results (what I asked about if the gradient can start from another point)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an example where this is useful? More to the point, if we ever move to react-native-gradients - it supports angle and not start & end

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

example:

        <View width={80} height={80}>
          <LinearGradient
            colors={[Colors.$backgroundPrimaryHeavy, Colors.$backgroundPrimaryMedium, Colors.$backgroundPrimaryLight]}
            start={{x: 0.01, y: 0.02}}
            end={{x: 0.9, y: 0.8}}
          >
            <View width={80} height={80} center>
              <Text white>Hello</Text>
            </View>
          </LinearGradient>
        </View>

As you can see the gradient as more of the backgroundPrimaryLight since the start coordinates changed.

Does the react-native-gradients migration will be soon ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically we can get something very similar using a different colors array (for example: const COLORS = [Colors.$backgroundPrimaryHeavy, Colors.$backgroundPrimaryMedium, Colors.$backgroundPrimaryMedium];)

Either way I don't want to add a general support for start and end, if it'll be required we can add a specific type for it (so we can deprecate it more easily).

Not sure we'll do it, it does't seem like it's very active, but it's only a few JS files, so we can more easily handle changes if we need to.


type CommonGradientProps = Pick<LinearGradientProps, 'colors' | 'children'> & {
angle?: number;
center?: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The center alignment props only affect the children content, not the gradient.
These should be moved to a separate prop group like ContentAlignmentProps with boolean values, WDYT ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't center always affects the children?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it does, for me when I pass center or centerV/H to the gradient I expect the color of the gradient to be aligned also.
I leave it up to you.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've changed the screen's text but I think the modifier is ok

const {type = 'rectangle', ...others} = props;

useEffect(() => {
if (LinearGradient === undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This useEffect isn't relevant because the fallback is {default: () => null}, not undefined.
Am I missing something ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I'm reverting the change to LinearGradientPackage since it affects SkeletonView as well

}
}, []);

if (!LinearGradient) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if (!LinearGradient) check is irrelevant because LinearGradient cannot be falsy (it's either a component or a function).
The fallback function () => null will pass this check consider checking if (LinearGradient === null) instead.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed a little

@@ -16,24 +16,29 @@ import {
View
} from 'react-native-ui-lib';

interface RadioGroupOptions {
interface StateOptions {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file includes changes that belong in a different PR.
Have you verified that these changes don't break other screens ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes are required for this PR, if I made these changed in another (clean) PR, how would we test them?
Tested a few screens

Copy link
Contributor

@adids1221 adids1221 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, Iv'e left few comments.

@adids1221 adids1221 assigned M-i-k-e-l and unassigned adids1221 Aug 21, 2025
@M-i-k-e-l M-i-k-e-l assigned adids1221 and unassigned M-i-k-e-l Aug 24, 2025
@adids1221 adids1221 assigned M-i-k-e-l and unassigned adids1221 Aug 25, 2025
@M-i-k-e-l M-i-k-e-l assigned adids1221 and unassigned M-i-k-e-l Sep 2, 2025
Copy link
Contributor

@adids1221 adids1221 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 🎨

@adids1221 adids1221 merged commit fc02adc into master Sep 3, 2025
1 check passed
@adids1221 adids1221 deleted the feat/incubator-gradient branch September 3, 2025 10:34
@adids1221 adids1221 added the hotfix Requires a hotfix release label Sep 3, 2025
adids1221 pushed a commit that referenced this pull request Sep 4, 2025
* Incubator.Gradient - new component

* Oops

* Revert LinearGradientPackage change

* Improve screen
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hotfix Requires a hotfix release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants